BUGFIX: Copy node respect tethered nodes correctly #5350 #5358
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rtions made (a little)
> Node aggregate classifications do not match. Expected "regular", got "tethered". Or a catchup will fail with > The NodeName must be set if the Node is tethered
In case a leaf node that is tethered is copied, we un-tether it. A migration `flow migrateevents:migratecopytetherednode` fixes this for previous cases. In case a tethered node is attempted to be copied which has tethered childnodes determined by the parent nodetype we fail. This is not possible. What we dont do yet is determine this correctly and there are false positives: > we assume here that the child node is tethered because the grandparent specifies that. > this is not always fully correct and we could loosen the constraint by checking the node type schema to correctly determine this, we have to evaluate the node type schema which is not available currently.
mhsdesign
commented
Nov 10, 2024
Comment on lines
51
to
+56
| public static function fromSubgraphAndStartNode(ContentSubgraphInterface $subgraph, Node $sourceNode): self | ||
| { | ||
| $childNodes = []; | ||
| return self::createSnapshotRecursively($subgraph, $sourceNode, true); | ||
| } | ||
|
|
||
| private static function createSnapshotRecursively(ContentSubgraphInterface $subgraph, Node $sourceNode, bool $firstLevel): self |
There was a problem hiding this comment.
jup this looks ugly but it doesnt matter, this is just meant as a hotfix. In order to evaluate constraints correctly (64) and also copying of dimensions we have to move this logic to a command handler imo and create a serialised version of this stuff.
But first things first, hey at least copying is now a little les broken :D
There was a problem hiding this comment.
FYI: I created an issue for that: #5359
|
Superseeded by #5371 we want copy nodes as a service instead :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #5350
In case a leaf node that is tethered is copied, we un-tether it.
A migration
flow migrateevents:migratecopytetherednodefixes this for previous cases.In case a tethered node is attempted to be copied which has tethered childnodes determined by the parent nodetype we fail. This is not possible. What we dont do yet is determine this correctly and there are false positives:
to correctly determine this, we have to evaluate the node type schema which is not available currently.
Upgrade instructions
Review instructions
Checklist
FEATURE|TASK|BUGFIX!!!and have upgrade-instructions